Skip to content

Fallback to default fragment implementation if no candidates found #3018

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

quaff
Copy link
Contributor

@quaff quaff commented Jan 5, 2024

It's an enhancement that should not break existing behaviors.

The default implementation is $FragmentInterfaceName + $ImplementationPostfix, for example com.example.FragmentImpl is the default implementation of com.example.Fragment.

It's useful for sharing repository fragments as library, application doesn't have to include library package in @Enable…Repositories, and it will back off if application provides custom implementation.

See spring-projects/spring-data-jpa#3287

It's an enhancement that should not break existing behaviors.

The default implementation is `$FragmentInterfaceName + $ImplementationPostfix`, for example `com.example.FragmentImpl` is the default implementation of `com.example.Fragment`.

It's useful for sharing repository fragments as library, application doesn't have to include library package in `@Enable…Repositories`, and it will back off if application provides custom implementation.

See spring-projects/spring-data-jpa#3287
@mp911de
Copy link
Member

mp911de commented Mar 4, 2024

Thanks for your suggestion. We need to discuss in the team whether we want to support external fragments and how discovery is affected with a fragment implementation inside the base package vs. a fragment implementation that resides in the package the interface is defined in.

@quaff
Copy link
Contributor Author

quaff commented Aug 25, 2024

Any thoughts?

@mp911de
Copy link
Member

mp911de commented Aug 26, 2024

We've introduced #3090 to register fragments by specifying implementations in spring.factories. Falling back to a same-package default implementation deviates from what Spring's approach and seems a bit random.

Care to check out #3090? It allows you to specify an implementation for a fragment interface. Once the fragment interface is used in a repository, you will get the fragment implementation that is associated with it.

@mp911de mp911de added status: waiting-for-feedback We need additional information before we can continue and removed for: team-attention An issue we need to discuss as a team to make progress labels Aug 26, 2024
@quaff
Copy link
Contributor Author

quaff commented Aug 26, 2024

Falling back to a same-package default implementation deviates from what Spring's approach and seems a bit random.

Same-package default implementation is unique, could you explain why it's random?
Specifying it in spring.factories deviates from "Convention over Configuration" principle.


org.springframework.data.repository.config.spifragment.SpiFragment=org.springframework.data.repository.config.spifragment.SpiFragmentImpl

in spring.factories could be omitted after this commit is merged.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 26, 2024
@mp911de
Copy link
Member

mp911de commented Aug 26, 2024

Specifying it in spring.factories deviates from "Convention over Configuration" principle.

The opposite is true. We're adding means of configuration for arrangements that require a more explicit configuration.

Scanning outside the base package would strongly deviate from our conventions. By broadening scanning locations to interfaces imported into a repository declaration we would ignore configuration.

That being said, we're closing this PR in favor of #3090.

@mp911de mp911de closed this Aug 26, 2024
@mp911de mp911de added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants